Skip to content

docs(#5586): add security guidance for destructive forge operations - #5587

Merged
rh-hemartin merged 1 commit into
mainfrom
agent/5586-forge-destructive-ops-guidance
Jul 27, 2026
Merged

docs(#5586): add security guidance for destructive forge operations#5587
rh-hemartin merged 1 commit into
mainfrom
agent/5586-forge-destructive-ops-guidance

Conversation

@fullsend-ai-coder

@fullsend-ai-coder fullsend-ai-coder Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a "Security considerations for destructive operations" section to docs/contributing/forge-abstraction.md
  • Covers three principles for callers of destructive forge.Client methods: verify ownership before acting, prefer fail-closed defaults, and guard against predictable-name attacks
  • References closeStaleScaffoldPRs in internal/layers/commit.go as a positive example of all three principles
  • Includes review guidance to flag destructive operations that lack authorization checks

Motivation

PR #5478 added CloseChangeProposal to the forge interface without authorization checks, requiring a review-fix cycle to add author verification. The forge-abstraction guide had no guidance on security considerations for destructive operations, so the code agent had no signal that such checks were needed.

Testing

  • make lint-md-links passes — no broken links introduced
  • Documentation-only change; no Go code or tests affected

Closes #5586

Post-script verification

  • Branch is not main/master (agent/5586-forge-destructive-ops-guidance)
  • Secret scan passed (gitleaks — 253eb62573e4783735126d34c6283fecde9f0d71..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@fullsend-ai-coder
docs(#5586): add security guidance for destructive forge operations
b90652b 
Add a 'Security considerations for destructive operations' section
to docs/contributing/forge-abstraction.md. The section covers three
principles for callers of destructive forge methods (close, delete,
merge, force-push): verify ownership before acting, prefer
fail-closed defaults when authorization data is missing, and guard
against predictable-name attacks where external actors could create
resources on well-known branch paths.

References closeStaleScaffoldPRs in internal/layers/commit.go as a
positive example of all three principles. Includes review guidance
to flag destructive operations missing authorization checks.

Closes #5586
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 25, 2026 00:59
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Jul 25, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:01 AM UTC · Completed 1:11 AM UTC
Commit: b90652b · View workflow run →

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown

Site preview

Preview: https://6990d6f8-site.fullsend-ai.workers.dev

Commit: b90652b52a45ec2540be5f233b9cae67a935d3cc

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

Looks good to me

Labels: PR adds documentation to docs/contributing/forge-abstraction.md

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/docs User-facing documentation labels Jul 25, 2026
@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 27, 2026
Merged via the queue into main with commit 206a1e5 Jul 27, 2026
30 of 31 checks passed
@rh-hemartin
rh-hemartin deleted the agent/5586-forge-destructive-ops-guidance branch July 27, 2026 08:49
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:51 AM UTC · Completed 9:02 AM UTC
Commit: b90652b · View workflow run →

@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 27, 2026

Copy link
Copy Markdown

Retro: PR #5587 — add security guidance for destructive forge operations

Workflow timeline: Retro on PR #5478 filed issue #5586 (Jul 25 00:48 UTC) → Triage run 30137151836 (~3 min, labeled docs/security/medium/ready-to-code) → Code run 30137317007 (~6.5 min, single commit b90652b, PR created) → Review run 30137571215 (~12 min, $2.72, 4 sub-agents, 0 findings, APPROVE + ready-for-merge) → Human rh-hemartin approved (Jul 27 08:40 UTC) → Merged (Jul 27 08:49 UTC).

Total agent time: ~22 minutes. Wall-clock to merge: ~56 hours (55+ hours waiting for human review).

What went well

  1. Complete retro-to-fix cycle. The full pipeline (retro analysis → issue filing → triage → code → review → human approval → merge) worked end-to-end. The retro agent on PR fix(#5475): close stale scaffold PRs when switching install modes #5478 identified a genuine documentation gap, filed a well-scoped issue, and the code agent implemented it correctly on the first attempt.

  2. Zero rework. The code agent produced a clean, well-structured 14-line documentation addition with a single commit. No fix cycles were needed. This is directly attributable to the quality of the retro-generated issue — it specified the target file, section structure, principles to cover, and a positive code example.

  3. Appropriate review depth. The review agent correctly approved the docs-only PR. The correctness sub-agent (Opus) verified that the cited code reference (closeStaleScaffoldPRs in internal/layers/commit.go) accurately implements all three stated principles. The review correctly skipped irrelevant dimensions (security, docs-currency, cross-repo-contracts).

  4. Triage efficiency. The triage agent labeled the issue correctly in ~3 minutes with appropriate category (docs/security), priority (medium), and routing (ready-to-code).

Evidence for existing issues

No proposals filed

All identified improvement opportunities are covered by existing open issues. The workflow executed cleanly with zero rework, and the retro-to-fix pipeline demonstrated its intended value by closing a real documentation gap that had caused an extra review-fix cycle on PR #5478.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rh-hemartin rh-hemartin rh-hemartin approved these changes
+1 more reviewer
@fullsend-ai-review fullsend-ai-review[bot] fullsend-ai-review[bot] approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component/docs User-facing documentation ready-for-merge All reviewers approved — ready to merge ready-for-review Agent PR ready for human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add authorization guidance for destructive forge operations to forge-abstraction.md

1 participant

@rh-hemartin